fix(operations): replace the component removal recipe that undoes itself - #684

[MINOR] Warning's named CRD list undercounts the blast radius under metallb.frrk8s.enabled: true

The alert names IPAddressPool, L2Advertisement, BGPPeer as the destroyed kinds. packages/system/metallb/charts/metallb/charts/frr-k8s/charts/crds/templates/*.yaml renders four more CRDs (frrconfigurations, frrnodestates, bgpsessionstates, frrk8sconfigurations) under a different group, frrk8s.metallb.io, as a nested subchart of the same cozystack.metallb release, and metallb.frrk8s.enabled: true is a documented toggle. The preceding generic sentence already covers this correctly, so this is not wrong, just incomplete for BGP-mode operators reading only the named list. Same text is duplicated identically at v1.1:78, v1.2:83, v1.3:83, v1.4:83, v1.5:83, v1.6:83.

Dropped the kind list. The sentence now points at the chart and says subcharts count, so it does not need updating per component.

One scope correction: frr-k8s only landed in packages/system/metallb at v1.5.0, so v1.1 through v1.4 never shipped those four CRDs. The new wording holds on all of them anyway.

[MINOR] No signal for when the destructive uninstall has actually finished

internal/operator/package_reconciler.go registers no finalizer on Package (verified: the only finalizer usage in the tree belongs to an unrelated tapmaterializer reconciler), so kubectl delete package.cozystack.io returns as soon as the Package object itself is gone. The owned HelmRelease's deletion, and the helm uninstall inside it that destroys the CRDs/namespaces the Warning describes, proceeds asynchronously afterward via the ownerReference cascade. The recipe has no follow-up check for confirming the destructive step is actually done. Same text is duplicated identically at v1.1:84, v1.2:89, v1.3:89, v1.4:89, v1.5:89, v1.6:89.

Added a wait after the delete:

kubectl wait --for=delete helmrelease/<component> --namespace <namespace> --timeout=10m

with a line saying why the delete returns early.

[MAJOR] the completion check cannot fail on a wrong guess, and its two placeholders are underivable

kubectl wait --for=delete suppresses not-found unconditionally, so a name the reader guessed wrong exits 0 and prints nothing, which is indistinguishable from the object having been removed. In kubectl v1.34.1:

// pkg/cmd/wait/wait.go:370-372, 379-380
isForDelete := strings.ToLower(o.ForCondition) == "delete"
if visitor, ok := visitor.(*resource.Result); ok && isForDelete {
	visitor.IgnoreErrors(apierrors.IsNotFound)
}
...
if visitCount == 0 && !isForDelete {
	return errNoMatchingResources

There is no --ignore-not-found to turn that off, and the asymmetry is the trap: a real object prints condition met, a wrong one prints nothing and still exits 0.

The reader's only handle is the Package name from kubectl get packages, e.g. cozystack.metallb. Neither placeholder follows from it. The HelmRelease name and namespace come from the PackageSource install block, which this page never mentions:

$ git show origin/main:packages/core/platform/sources/metallb.yaml | sed -n '10,25p'
    - name: metallb
      path: system/metallb
      install:
        privileged: true
        namespace: cozy-metallb
        releaseName: metallb

There is also no naming rule to fall back on, because one Package can render several releases. And the namespace a reader is most likely to substitute is the cozy-system printed on this page's own Package example.

The operator already labels every HelmRelease it generates with the Package that owns it, at every tag from v1.0.8 to main:

$ git grep -n 'cozystack.io/package' origin/main -- internal/operator/package_reconciler.go
264:		labels["cozystack.io/package"] = pkg.Name
$ for t in v1.0.8 v1.1.7 v1.3.6 v1.6.3; do git grep -c 'cozystack.io/package' $t -- internal/operator/package_reconciler.go; done
2
2
2
2

So the step before the delete can produce both values and cover a multi-release Package at the same time:

kubectl get helmrelease --all-namespaces --selector cozystack.io/package=<package-name>

Fixed in f621e84. The step before the delete lists the releases the Package owns and prints namespace, name and spec.suspend as columns, so both values come out of the listing and a Package that renders several releases is covered.

kubectl get helmrelease --all-namespaces --selector cozystack.io/package=<package-name> \
  --output custom-columns='NAMESPACE:.metadata.namespace,NAME:.metadata.name,SUSPENDED:.spec.suspend'

The line under the wait says --for=delete exits 0 for a name that was never there, so the values have to come from the listing rather than a guess. v1.0 gets the same listing, taken before the values edit, because there the Package goes away with the component.

[MAJOR] "Flux uninstalls the release when the HelmRelease goes away" is conditional, and Cozystack reaches the other branch itself

The paragraph states this flat, and the wait above rests on it. The pinned controller is ghcr.io/fluxcd/helm-controller:v1.5.0 (internal/fluxinstall/manifests/fluxcd.yaml:8097), and at that tag the uninstall is gated on suspend while the finalizer removal is not:

// helm-controller v1.5.0, internal/controller/helmrelease_controller.go
func (r *HelmReleaseReconciler) reconcileDelete(ctx context.Context, obj *v2.HelmRelease) (ctrl.Result, error) {
	// Only uninstall the release and delete the HelmChart resource if the
	// resource is not suspended.
	if !obj.Spec.Suspend {
		if err := r.reconcileReleaseDeletion(ctx, obj); err != nil {
			return ctrl.Result{}, err
		}
		...
	}

	if !obj.DeletionTimestamp.IsZero() {
		controllerutil.RemoveFinalizer(obj, v2.HelmReleaseFinalizer)

Suspending is routine, and Cozystack gets there on its own rather than only by an operator running flux suspend:

$ git grep -n 'Spec.Suspend = true\|Spec.Suspend = existing' origin/main -- internal/
internal/controller/fluxplunger/flux_plunger.go:258:	latestHR.Spec.Suspend = true
internal/operator/package_reconciler.go:454:	hr.Spec.Suspend = existing.Spec.Suspend

The plunger sets it, and the operator carries it across every subsequent update. The two halves compound with the finding above: the admin deletes the Package, the HelmRelease disappears in milliseconds with nothing uninstalled, and the wait reports the destructive part finished while every workload, CRD and custom resource is still there. Nothing on the page lets the reader tell that apart from success. One sentence telling them to check spec.suspend first, and that a returned wait is not by itself proof, would close it.

Fixed in f621e84. The same if !obj.Spec.Suspend is in helm-controller v1.4.3, which v1.0 through v1.4 run, so it applies to every page in this PR and not only the ones on v1.5.0. The listing before the delete prints SUSPENDED and the page says to clear it first; the wait paragraph now ends with "a returned wait says the HelmRelease is gone, not that the uninstall ran". The PR body carried the same flat claim and is rewritten.

[MAJOR] resource-policy: keep is not the whole survival rule from v1.5.0, and this PR's own edit is the counterexample

The Warning gives the annotation as the only thing that keeps an object alive through the uninstall. Since v1.5.0 there is a second, independent mechanism, and the rule as stated predicts the opposite outcome for the object this PR just removed from that sentence.

$ git show origin/main:packages/core/platform/templates/deletion-protection.yaml
  ValidatingAdmissionPolicy that blocks DELETE on any object carrying the
  platform.cozystack.io/no-delete=true label.
...
  validations:
  - expression: "false"
...
  validationActions: [Deny]
$ git log --format=%H -1 --diff-filter=A -- packages/core/platform/templates/deletion-protection.yaml
0c17d1870cef5a51a891928730d470ea556b1c8d
$ git tag --contains 0c17d1870 | sort -V | head -1
v1.5.0

The tenant-root Namespace carries that label and no keep (the keep a few lines below is on the sibling HelmRelease), which is exactly why the v1.5, v1.6 and next copies correctly drop it from the list one sentence after a rule saying it should go:

$ git show origin/main:packages/system/cozystack-basics/templates/tenant-root.yaml | head -8
kind: Namespace
metadata:
  name: tenant-root
  labels:
    platform.cozystack.io/no-delete: "true"

It matters past that one example, because the rule is what a reader applies to other components. Two more labelled-and-unkept objects belong to removable packages:

$ for f in packages/system/linstor/templates/cluster.yaml packages/system/cert-manager-issuers/templates/cluster-issuers.yaml; do
>   echo "$f no-delete=$(git show origin/main:$f | grep -c no-delete) keep=$(git show origin/main:$f | grep -c resource-policy)"; done
packages/system/linstor/templates/cluster.yaml no-delete=1 keep=0
packages/system/cert-manager-issuers/templates/cluster-issuers.yaml no-delete=3 keep=0

An operator removing cozystack.linstor or cozystack.cert-manager concludes from this page that those go with the release. What they get is an admission denial partway through the uninstall, a different end state with a different recovery. Naming the label next to the annotation, and pointing at the documented bypass (kubectl label <kind> <name> platform.cozystack.io/no-delete-), is enough. The v1.0 to v1.4 copies are accurate as written.

Fixed in f621e84, on v1.5, v1.6 and next only, since v1.0 through v1.4 predate the policy. The Warning names the label next to the annotation, gives kubectl get <kind> --all-namespaces --selector platform.cozystack.io/no-delete=true for finding what carries it, and kubectl label <kind> <name> --namespace <ns> platform.cozystack.io/no-delete- for taking it off, with the consequence spelled out. cozystack.cozystack-basics labels the tenant-root Namespace and the tenant-root HelmRelease, the HelmRelease is kept by its annotation, and unlabelling the Namespace hands the root tenant to the uninstall. The label has to come off every labelled object in a release, which the cert-manager-issuers release of cozystack.cert-manager makes concrete with three ClusterIssuers.

Chasing your point further turned up a worse outcome than "the object stays". Uninstall.Run bails on any error out of deleteRelease (pkg/action/uninstall.go:122-125), and reconcileReleaseDeletion passes that error up, so the finalizer stays on for as long as the denial does. So removing cozystack.cozystack-basics on v1.5+ leaves the HelmRelease sitting in deletion and the kubectl wait on the page runs to its timeout. The Warning says that now, not just that the object survives.

[MAJOR] the v0 tree keeps the bare command and gets none of the warning

Out of this diff, and pre-existing, so a follow-up PR is a fine resolution. Raising it here because it is the same gap you folded v1.0 in to close last round, and v0 is the last tree still carrying the original line:

$ grep -n 'kubectl delete hr' content/en/docs/v0/operations/configuration/components.md
66:kubectl delete hr -n <namespace> <component>
$ grep -c '^content/en/docs/v0/' /tmp/...-files.txt
0

The page is published and carries no deprecation banner:

$ sed -n '236,239p' hugo.yaml
    - version: "v0"
      url: "/docs/v0/"
      id: "v0"
      order: 1

The destruction the Warning describes belongs to helm uninstall, not to Package objects, so it applies to v0 in full: at v0.41.11 the MetalLB chart renders its CRDs as ordinary templates with no resource-policy, and a v0 reader following line 66 loses every IPAddressPool, L2Advertisement and BGPPeer cluster-wide. "v0 predates Package objects" excuses the recipe, not the missing warning.

Folded in here rather than deferred, at f621e84. v0 keeps its recipe and gets the warning ahead of the command. I checked charts/metallb/charts/crds/templates/crds.yaml at v0.1.0, v0.10.0, v0.20.0, v0.30.0 and v0.41.11: no resource-policy at any of them, so the destruction is the same on every 0.x line. The v0 copy drops the cozystack.cozystack-basics half of the sentence, that package does not exist before v1.

[MINOR] kubectl get package cannot list a name that is not enabled yet

The sentence offers kubectl get package as the discovery step for both enabledPackages and disabledPackages, and it only answers for the second. An optional package has no Package object until its name is already in enabledPackages (packages/core/platform/templates/_helpers.tpl renders it under if and (has $name $enabled) (not (has $name $disabled))), and cozystack.hetzner-robotlb, the worked example right below, is one of those. PackageSources are rendered unconditionally from sources/*.yaml, so kubectl get packagesource answers for both lists.

The same wording is already on v1.2 through next unchanged, so fixing it is a sweep rather than a change to this hunk.

Separately on the v1.0 page: "Disabling components must be done before installing Cozystack." still sits directly above the new paragraph explaining that the same edit removes an already-installed component.

Fixed in f621e84, swept across v1.1 through next rather than only the hunk. The discovery step is kubectl get packagesource now, with a clause saying kubectl get package answers only for disabledPackages. The and (has $name $enabled) (not (has $name $disabled)) gate is in _helpers.tpl from v1.1.0 on, and templates/sources.yaml globs sources/*.yaml unconditionally at all of those tags. The v1.0 line you noted at the end is gone as well: that page opens with "Disabling a component before installing Cozystack keeps it out of the installation entirely".